Skip to content

refactor(db): remove function for periodic backup of RocksDB database#6724

Merged
kuny0707 merged 6 commits intotronprotocol:developfrom
317787106:feature/remove_db_backup
May 9, 2026
Merged

refactor(db): remove function for periodic backup of RocksDB database#6724
kuny0707 merged 6 commits intotronprotocol:developfrom
317787106:feature/remove_db_backup

Conversation

@317787106
Copy link
Copy Markdown
Collaborator

@317787106 317787106 commented Apr 28, 2026

What does this PR do?

Removes the periodic database backup feature (storage.backup) entirely. This includes the backup configuration class, the AspectJ aspect that intercepted pushBlock to trigger copies, the backup utility, and all associated test fixtures and config files.

Closes #6595.

Why are these changes required?

The storage.backup feature was designed to guard against data corruption from disk failures or abrupt process termination (e.g. kill -9). It has since become more harmful than helpful:

  1. The original threat no longer applies. Extensive testing has confirmed that kill -9 does not corrupt RocksDB, which was the primary motivation for the feature.
  2. Copy times block block synchronisation. As of early 2026, the mainnet state database is close to 3 TB. A single backup copy can take several hours, during which the node cannot sync blocks — a fatal risk to service stability.
  3. A better alternative exists. Operators can deploy a primary–backup pair of full nodes using node.backup (UDP keep-alive failover). If one node becomes unavailable, traffic can be switched to the standby without any file-copy disruption.

Changes:

  • Deleted DbBackupConfig, BackupDbUtil, BackupRocksDBAspect, NeedBeanCondition (common / framework).
  • Removed CommonParameter.dbBackupConfig field and Args.initRocksDbBackupProperty() initialiser.
  • Removed ConfigKey.STORAGE_BACKUP_* constants and the conditional backupRocksDBAspect Spring bean from DefaultConfig.
  • Deleted BackupDbUtilTest and config-test-dbbackup.conf; updated RocksDbDataSourceImplTest and SupplementTest to use TEST_CONF.
  • Configuration change: the storage.backup block is no longer read and has no effect. Nodes carrying this section in their config file can leave it in place without error, but it will be silently ignored.

Backward Compatibility: Not compatible with v4.8.1 or older.

This PR has been tested by:

  • Unit Tests

Follow up

Operators who currently rely on storage.backup should migrate to the node.backup primary–backup failover configuration.

Extra details

N/A

@halibobo1205 halibobo1205 added the topic:DB Database label Apr 29, 2026
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.2 milestone Apr 29, 2026
halibobo1205
halibobo1205 approved these changes May 8, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@halibobo1205 halibobo1205 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

xxo1shine
xxo1shine reviewed May 9, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@xxo1shine xxo1shine left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD] framework/build.gradle still references BackupDbUtilTest.class in two spots, but this PR deletes that test class:

  • line 119: t.exclude '**/BackupDbUtilTest.class' (Windows-exclude block)
  • line 164: include 'org/tron/core/db/backup/BackupDbUtilTest.class' (in the testWithRocksDb task)

Gradle silently ignores include/exclude patterns that match nothing, so the build won't break — but they're dead config. Please remove them in this PR so the cleanup is complete.

Sunny6889
Sunny6889 approved these changes May 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Sunny6889 Sunny6889 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operator notice (suggest adding to release notes)

Nodes upgrading from a version where storage.backup.enable = true will have leftover artifacts on disk:

  • The propPath file configured by the user (default: prop.properties)
  • The bak1path / bak2path directories (defaults: bak1/database/ and bak2/database/)

Please call this out explicitly in the release notes: operators should manually delete these paths after upgrading to reclaim disk space. Without this guidance, the dead backup directories will silently keep consuming storage on every node that previously had the feature enabled.

@317787106
Copy link
Copy Markdown
Collaborator Author

317787106 commented May 9, 2026

@Sunny6889 This feature stopped working a long time ago. It might be worth mentioning this in the release notes documentation.

@317787106
Copy link
Copy Markdown
Collaborator Author

317787106 commented May 9, 2026

[SHOULD] framework/build.gradle still references BackupDbUtilTest.class in two spots, but this PR deletes that test class:

  • line 119: t.exclude '**/BackupDbUtilTest.class' (Windows-exclude block)
  • line 164: include 'org/tron/core/db/backup/BackupDbUtilTest.class' (in the testWithRocksDb task)

Gradle silently ignores include/exclude patterns that match nothing, so the build won't break — but they're dead config. Please remove them in this PR so the cleanup is complete.

Remove it in framework/build.gradle, thanks.

@kuny0707 kuny0707 merged commit 0757279 into tronprotocol:develop May 9, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this to Done in java-tron May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xxo1shine xxo1shine xxo1shine left review comments

@halibobo1205 halibobo1205 halibobo1205 approved these changes

@Sunny6889 Sunny6889 Sunny6889 approved these changes

Assignees

No one assigned

Labels

topic:DB Database

Projects

java-tron
Status: Done

Milestone

GreatVoyage-v4.8.2

Development

Successfully merging this pull request may close these issues.

[Feature] Remove periodic database backup in favor of dual-node failover

5 participants

@317787106 @xxo1shine @halibobo1205 @Sunny6889 @kuny0707